Skip to content

PHPLIB-1123: Use array_is_list #1093

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 13, 2023
Merged

PHPLIB-1123: Use array_is_list #1093

merged 2 commits into from
Jun 13, 2023

Conversation

GromNaN
Copy link
Member

@GromNaN GromNaN commented Jun 1, 2023

Fix PHPLIB-1123

Use the new function from PHP 8.1 to simplify code and optimise performances.

This PR will conflict with #1078

@@ -241,18 +242,14 @@ public function __construct(string $databaseName, string $collectionName, array
}

if (isset($options['pipeline'])) {
$expectedIndex = 0;
if (! array_is_list($options['pipeline'])) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like psalm is missing that $options['pipeline'] is an array by this point. This might require a local variable assignment and @var annotation to overcome.

Alternatively, this might overlap with #1078 for PHPLIB-881.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

phpcs dictate me to use assert. This will be reverted by #1078.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strange that this was required given we throw earlier for:

if (isset($options['pipeline']) && ! is_array($options['pipeline']))

Glad we'll be able to revert this, though.

@GromNaN GromNaN changed the title PHPLIB-113: Use array_is_list PHPLIB-1123: Use array_is_list Jun 7, 2023
@GromNaN GromNaN merged commit 7e63041 into mongodb:master Jun 13, 2023
@GromNaN GromNaN deleted the PHPLIB-1123 branch June 13, 2023 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants